Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Bug Fix] SEGV on query with Left Outer Join (#7787) #7901

Merged
merged 1 commit into from
Feb 18, 2025

Conversation

colm-mchugh
Copy link
Contributor

@colm-mchugh colm-mchugh commented Feb 14, 2025

DESCRIPTION: Fixes a crash in left outer joins that can happen when there is an an aggregate on a column from the inner side of the join.

Fix the SEGV seen in #7787 and #7899; it occurs because a column in the targetlist of a worker subquery can contain a non-empty varnullingrels field if the column is from the inner side of a left outer join. The issue can also occur with the columns in the HAVING clause, and this is also tested in the fix. The issue was triggered by the introduction of the varnullingrels to Vars in Postgres 16 (2489d76c)

There is a related issue, #7705, where a non-empty varnullingrels was incorrectly copied into the query tree for the combine query. Here, a non-empty varnullingrels field of a var is incorrectly copied into the query tree for a worker subquery.

The regress file from #7705 is used (and renamed) to also test this (#7787). An alternative test output file is required for Postgres 15 because of an optimization to DISTINCT in Postgres 16 (1349d2790bf).

Fix the SEGV seen in #7787; it occurs because a column in the targetlist
of a worker subquery can contain a non-empty varnullingrels field if the
column is from the inner side of a left outer join.
The issue can also occur with the columns in the HAVING clause, and this
is also tested in the fix. The issue was triggered by the introduction
of the varnullingrels to Vars in Postgres 16 (2489d76c)

There is a related issue, #7705, where a non-empty varnullingrels
was incorrectly copied into the query tree for the combine query.
Here, a non-empty varnullingrels field of a var is incorrectly copied
into the query tree for a worker subquery.

The regress file from #7705 is used (and renamed) to also test this
(#7787). An alternative test output file is required for Postgres 15
because of an optimization to DISTINCT in Postgres 16 (1349d2790bf).
Copy link

codecov bot commented Feb 14, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 89.45%. Comparing base (c55bc8c) to head (9ae7666).
Report is 19 commits behind head on release-13.0.

Additional details and impacted files
@@               Coverage Diff                @@
##           release-13.0    #7901      +/-   ##
================================================
- Coverage         89.48%   89.45%   -0.03%     
================================================
  Files               276      276              
  Lines             60063    59893     -170     
  Branches           7524     7510      -14     
================================================
- Hits              53747    53579     -168     
- Misses             4166     4169       +3     
+ Partials           2150     2145       -5     

Copy link
Member

@onurctirtir onurctirtir left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's please also add a line that starts with DESCRIPTION: that contains a shorter description that we can include in the release changelog, like,

DESCRIPTION: Fixes a crash with outer joins that happens when ..

Copy link
Member

@naisila naisila left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very important fix, thanks!

It's something we have missed when we accounted for the varnullingrels addition in PG16.

After merging, can we make sure that all relevant issues are closed?
Also, this needs to be backported to 12.1.

@naisila naisila removed the request for review from m3hm3t February 18, 2025 11:38
@colm-mchugh colm-mchugh merged commit 8f3d9de into release-13.0 Feb 18, 2025
119 checks passed
@colm-mchugh colm-mchugh deleted the colm/issue_7787 branch February 18, 2025 12:41
colm-mchugh added a commit that referenced this pull request Feb 18, 2025
DESCRIPTION: Fixes a crash in left outer joins that can happen when
there is an an aggregate on a column from the inner side of the join.

Fix the SEGV seen in #7787 and #7899; it occurs because a column in the
targetlist of a worker subquery can contain a non-empty varnullingrels
field if the column is from the inner side of a left outer join. The
issue can also occur with the columns in the HAVING clause, and this is
also tested in the fix. The issue was triggered by the introduction of
the varnullingrels to Vars in Postgres 16 (2489d76c)

There is a related issue, #7705, where a non-empty varnullingrels was
incorrectly copied into the query tree for the combine query. Here, a
non-empty varnullingrels field of a var is incorrectly copied into the
query tree for a worker subquery.

The regress file from #7705 is used (and renamed) to also test this
(#7787). An alternative test output file is required for Postgres 15
because of an optimization to DISTINCT in Postgres 16 (1349d2790bf).
colm-mchugh added a commit that referenced this pull request Feb 18, 2025
DESCRIPTION: Fixes a crash in left outer joins that can happen when
there is an an aggregate on a column from the inner side of the join.

Fix the SEGV seen in #7787 and #7899; it occurs because a column in the
targetlist of a worker subquery can contain a non-empty varnullingrels
field if the column is from the inner side of a left outer join. The
issue can also occur with the columns in the HAVING clause, and this is
also tested in the fix. The issue was triggered by the introduction of
the varnullingrels to Vars in Postgres 16 (2489d76c)

There is a related issue, #7705, where a non-empty varnullingrels was
incorrectly copied into the query tree for the combine query. Here, a
non-empty varnullingrels field of a var is incorrectly copied into the
query tree for a worker subquery.

The regress file from #7705 is used (and renamed) to also test this
(#7787). An alternative test output file is required for Postgres 15
because of an optimization to DISTINCT in Postgres 16 (1349d2790bf).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants